Update/list view outline information about post #50414

n2erjo00 · 2023-05-06T14:19:18Z

What?

In the editors list view outline section added "Headings", "Paragraphs" and "Blocks" fields, which show total amount of mentioned element in the post.

Created new tab "Info" to house requested fields.

Also HTML structure changed from divs and spans to ul to improve HTML quality and accessibility

Why?

Issue was brought up in here #50280

How?

Added code which calculates the total values of the elements.
Changed structure of HTML to split content between two columns.

Testing Instructions

Open existing post (or page)
Open List view and then open Outline tab
Input new blocks, new paragraphs and headings
Open new post (don't save it)
Open List view and open Outline tab
Input new blocks, new paragraphs and headings

Testing Instructions for Keyboard

Screenshots or screencast

…d block count

…ucture to get content into two columns

alexstine

@n2erjo00 Thanks for the PR. I think this is a nice chance to make this section more accessible to screen reader users. Instead of using a visual 2 columns, let's use a table here instead? Visual columns are just that, visual columns.

n2erjo00 · 2023-05-11T04:48:11Z

@alexstine Good catch. One table coming up. I'll whip it up today at the evening

alexstine

@n2erjo00 Code looking good. One suggestion below.

@WordPress/gutenberg-design Might want to give this one a visual review for me?

alexstine · 2023-05-22T19:58:54Z

packages/edit-post/src/components/secondary-sidebar/list-view-outline.js

+				</caption>
+				<thead className="screen-reader-text">
+					<tr>
+						<th scope="col">{ __( 'Metric' ) }</th>


These are duplicated. Only need 2, not 4.

Mamaduka · 2023-05-23T12:13:59Z

packages/edit-post/src/components/secondary-sidebar/list-view-outline.js

+			return {
+				headingCount: getGlobalBlockCount( 'core/heading' ),
+				paragraphCount: getGlobalBlockCount( 'core/paragraph' ),
+				blockCount: blocks.length ? blocks.length : 0,


The TableOfContentsPanel panel uses getGlobalBlockCount here as well.

gutenberg/packages/editor/src/components/table-of-contents/panel.js

Lines 17 to 27 in 074443a

const { headingCount, paragraphCount, numberOfBlocks } = useSelect(

( select ) => {

const { getGlobalBlockCount } = select( blockEditorStore );

return {

headingCount: getGlobalBlockCount( 'core/heading' ),

paragraphCount: getGlobalBlockCount( 'core/paragraph' ),

numberOfBlocks: getGlobalBlockCount(),

};

},

[]

);

Mamaduka · 2023-05-23T12:18:52Z

I left a small note, but from a code perspective, this looks good.

This implementation uses tables instead of a list like the old TableOfContentsPanel component.

@afercia, you reviewed the original content structure PR. Was there a reason for choosing the list markup over tables?

afercia · 2023-07-11T08:59:38Z

Sorry I'm late to the party. I'd totally agree to add back the information about "Headings", "Paragraphs" and "Blocks" and I'd support this PR's effort, thank you! I don't see any good reason to remove important information.

@Mamaduka @alexstine regarding the table, I'm not sure this data can be represented correctly with a table. Well, it could be, but the table should have two columns: metrics and value. The table in this PR has 4 columns: metrics and value and again metrics and value. Actually, the 4 columns are used for visual purposes. Repeating metrics and value twice doesn't seem ideal to me.

For clarity, I copied and pasted the HTML output from this PR in a codepen at https://codepen.io/afercia/full/PoxOMLZ so that it can be tested more easily. Screenshot:

Aside:
The caption text Outline information about current post doesn't seem ideal to me:

This is not an outline.
We should not specify 'current post'.
In the previous implementation, this info was wrapped within a role="note" with an aria-label="Document Statistics". Maybe the role was a bit pointless but I'd use the Document Statistics terminology somewhere.

Overall, I'm not sure. a table is appropriate in this case. I'd just use a list. Worth mentioning the current HTML on trunk is just a bunch of divs and spans, which is totally unsemantic and, honestly, a poor document structure. A regression in semantics quality compared to WP 6.1.

Lastly, I'm not sure the info about Characters / Words / Time to read / Headings / Paragraphs / Blocks should live in the 'Outline' tab. This tab should only contain the Document outline. I'd consider a third tab dedicated to the statistics.

alexstine · 2023-07-14T18:28:27Z

@afercia Nice catch. I totally missed that. I just got really annoyed with how this info was being displayed so thought a table could make sense. At least the table could give us data relationships vs. a bunch of divs/spans for a fake list.

afercia · 2023-07-18T08:26:02Z

@alexstine thanks. When you have a chance, what about this point?

Lastly, I'm not sure the info about Characters / Words / Time to read / Headings / Paragraphs / Blocks should live in the 'Outline' tab. This tab should only contain the Document outline. I'd consider a third tab dedicated to the statistics.

I would like to make the 'Outline' tab contain only the document outline, in the first place.

alexstine · 2023-07-18T19:38:08Z

@afercia That's probably a fair point to make, just not sure what we would put in the Outline tab since List View tab is probably closer to an outline users would expect.

n2erjo00 · 2023-07-22T08:50:02Z

@afercia @alexstine Is the consensus that what we need is

Tab called "Info"
Data which was requested in the ticket should live in there in the form of <ul>
Current data which is printed in the "Outline" tab should be refactored into <ul>

afercia · 2023-07-24T07:34:05Z

@n2erjo00 thanks for the ping.
The new tab name Info or Statistics sounds good to me.
Not sure there's agreement about the other two points. I'd defer to @alexstine
To me, the simpler, the better.

alexstine · 2023-07-24T18:25:27Z

Sounds fine to me, as long as it is a proper <ul>.

…post

…component. These data fields will live in separate component

n2erjo00 · 2023-09-24T18:29:16Z

@alexstine Gentle nudge

alexstine · 2023-09-24T19:57:07Z

@n2erjo00 Sorry about this. I guess I forgot about this PR with WordCamp US happening. Can you give this a refresh from latest trunk? Packages have changed and my NPM is too new to build this now.

Thanks.

…post

n2erjo00 · 2023-09-25T10:16:50Z

@alexstine No worries even I had completely forgot the existence of this 😅 Latest trunk merged to this branch

alexstine

@n2erjo00 This is testing well. A couple fixes:

There is no space after the ":" so screen readers read out as "Characters:10" vs. "Characters: 10". Does not sound like a large issue until you listen to it for yourself. Is there a way you can add spacing, maybe outside of the translations? I'm not sure it is a good practice to include whitespace in translations but might be wrong about that.
The useInstanceId hook takes a second parameter, prefix. You could simplify the code. I left a comment with specific examples. Be sure to apply it to both files.

alexstine · 2023-09-26T02:40:43Z

packages/edit-post/src/components/secondary-sidebar/list-view-outline.js

-				<div>
+		<div className="edit-post-editor__list-view-overview__container">
+			<p
+				id={ `edit-post-editor-list-view-overview-outline-${ instanceId }` }


Suggested change

id={ `edit-post-editor-list-view-overview-outline-${ instanceId }` }

id={ instanceId }

alexstine · 2023-09-26T02:41:32Z

packages/edit-post/src/components/secondary-sidebar/list-view-outline.js

@@ -63,24 +64,37 @@ export default function ListViewOutline() {
 			headingCount: getGlobalBlockCount( 'core/heading' ),
 		};
 	}, [] );
+	const instanceId = useInstanceId( ListViewOutline );


Suggested change

const instanceId = useInstanceId( ListViewOutline );

const instanceId = useInstanceId( ListViewOutline, 'edit-post-editor-list-view-overview-outline' );

…readers read more clearly

…early. Simplified code by using second parameter of useInstance

alexstine

@n2erjo00 Thanks for all the hard work here. This is testing well for me.

Tagging @andrewserong for final review.

andrewserong · 2023-09-27T02:12:10Z

Thanks for the ping, and for the work here @n2erjo00!

Adding a new tab is a fairly prominent change, and I think it's a great idea listing out more stats about the number of different blocks within the document, whether that's included in the existing Outline tab, or in a new Info tab.

From playing around with the change, it looks like it could use a pass from a designer to ensure that the text is visually consistent with other areas of the app (e.g. spacing between the text and edge of the window, sizing, colours, etc), so I'll just add the Needs Design Feedback label to this PR.

Here's how the Outline and Info tabs are looking for me currently in testing:

Outline tab	Info tab

Nice work so far!

jameskoster · 2023-09-27T10:32:24Z

I don't know if there's enough 'information' to warrant an entire tab. Could that data live in the Outline tab? Here's a quick mockup:

Alternatively could all the data points live in the Info tab, leaving the Outline tab to concentrate solely on the outline?

n2erjo00 · 2023-09-27T16:27:35Z

@jameskoster Sure they could live in same tab 😄 Looking back at the discussions on this PR simpler the better was the reason to split between Outline and Info.

Based on the mockups I'd go with Outline and Info split. Since Outline already contains the heading structure (and it can eat a lot of space) putting black-and-white statics at the end would make Outline too crowded and little taxing to find them (if lot of headings).

I think I have time on upcoming weekend whip up the following

Remove following fields from the current "Outline" tab (Characters, Words, Time to read)
Add following fields to "Info" tab (Character, Words, Time to read)
Whip up some styles so the end result resembles closely to the mockups

…Text component

n2erjo00 · 2023-09-30T12:37:28Z

@jameskoster Did the things, honest but brutal feedback appreciated 😄

PS Mocks contained changes to "Multiple H1..." warning box. I did not implement these changes since they are not what this PR is for.
PSS Warning box design is much better than what we currently have 😄 I would love to see it some day in the trunk

andrewserong

Thanks for continuing on with this @n2erjo00! Code-wise it's looking pretty good to me.

In re-testing with some shorter blog-post-like content, I think it's pretty common to have posts that don't have very many headings, and in these cases, it means the Outline tab can be fairly empty. In playing around with this in my test environment, I think my personal preference would be to keep the Information as a footer section within the Outline tab as in James' mockup #50414 (comment). Having a separate tab for Info feels to me like it'd be more useful for posts that have very complex heading hierarchies, which I suspect are in the minority. And the footer idea for the Info section sounds like it could be a good way to mitigate that situation, too.

Here's a couple of screenshots of how it's looking for me:

Outline tab in a short post	Info tab in a short post

jameskoster · 2023-10-02T11:35:13Z

Mocks contained changes to "Multiple H1..." warning box. I did not implement these changes since they are not what this PR is for.

That's fine :)

Having a separate tab for Info feels to me like it'd be more useful for posts that have very complex heading hierarchies, which I suspect are in the minority

I tend to agree, the outline tab is often sparse, and the info tab isn't exactly dense. It would be nice to keep them combined.

I would like to make the 'Outline' tab contain only the document outline, in the first place.

Perhaps @afercia could elaborate on why the separate tabs are preferred. Is there an alternative such as labelling the tab "Outline & info", or even just "Info" if a case can be made that the outline is informational?

n2erjo00 · 2023-11-06T10:11:04Z

@afercia Gentle nudge. See comment above ☝️

n2erjo00 · 2024-02-22T07:33:29Z

@afercia Gentle nudgy-nudge-nudge

github-actions · 2024-02-22T07:33:42Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: n2erjo00 <n2erjo00@git.wordpress.org>
Co-authored-by: alexstine <alexstine@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Co-authored-by: andrewserong <andrewserong@git.wordpress.org>
Co-authored-by: afercia <afercia@git.wordpress.org>
Co-authored-by: jameskoster <jameskoster@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

n2erjo00 added 3 commits May 6, 2023 14:50

Added list-view outline to contain heading count, paragrapth count an…

e21624d

…d block count

Added code refactors to make it look nicer, changed slightly html str…

e638b1a

…ucture to get content into two columns

Updated styles to match new html structure

c6f1ca1

gziolo added [Type] Enhancement A suggestion for improvement. [Feature] Document Outline An option that outlines content based on a title and headings used in the post/page labels May 7, 2023

alexstine reviewed May 10, 2023

View reviewed changes

n2erjo00 added 2 commits May 11, 2023 18:38

Updated to use HTML table element for better screen reader access

83a69d2

Updated styles to match new table element

cd2e5e0

n2erjo00 requested a review from alexstine May 22, 2023 19:27

alexstine reviewed May 22, 2023

View reviewed changes

alexstine added the [Package] Edit Post /packages/edit-post label May 22, 2023

alexstine requested review from Mamaduka and a team May 22, 2023 20:01

Mamaduka reviewed May 23, 2023

View reviewed changes

alexstine added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Jul 24, 2023

n2erjo00 added 5 commits July 26, 2023 14:34

Merge branch 'trunk' into update/list-view-outline-information-about-…

94596f3

…post

Added new React component to show base information about the document

9c8a770

Removed table ui and heading, paragraphcount and blockCount from the …

60f8f2e

…component. These data fields will live in separate component

Added third tab to inserter called Info

16a9903

Style changes and additions since HTML uses <ul> structure

f70b5b5

n2erjo00 requested a review from alexstine August 4, 2023 16:09

Merge branch 'trunk' into update/list-view-outline-information-about-…

45186d4

…post

alexstine reviewed Sep 26, 2023

View reviewed changes

n2erjo00 added 2 commits September 26, 2023 18:52

Removed padding-right since added white-space to html to make screen-…

8efd549

…readers read more clearly

Added white-space to html element to make screen readers read more cl…

62d78cf

…early. Simplified code by using second parameter of useInstance

n2erjo00 requested a review from alexstine September 26, 2023 15:58

alexstine approved these changes Sep 27, 2023

View reviewed changes

alexstine requested a review from andrewserong September 27, 2023 02:00

alexstine removed the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Sep 27, 2023

andrewserong added the Needs Design Feedback Needs general design feedback. label Sep 27, 2023

n2erjo00 added 3 commits September 30, 2023 15:28

Added styles to overview list element

103cbee

Added CharacterCount, WordCount, TimeToRead to info tab

f46bd90

Removed CharacterCount, WordCount, TimeToRead from outline tab. Also …

9429354

…Text component

andrewserong reviewed Oct 1, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update/list view outline information about post #50414

Update/list view outline information about post #50414

n2erjo00 commented May 6, 2023 •

edited

Loading

alexstine left a comment

n2erjo00 commented May 11, 2023

alexstine left a comment

alexstine May 22, 2023

Mamaduka May 23, 2023

Mamaduka commented May 23, 2023

afercia commented Jul 11, 2023

alexstine commented Jul 14, 2023

afercia commented Jul 18, 2023

alexstine commented Jul 18, 2023

n2erjo00 commented Jul 22, 2023

afercia commented Jul 24, 2023

alexstine commented Jul 24, 2023

n2erjo00 commented Sep 24, 2023

alexstine commented Sep 24, 2023

n2erjo00 commented Sep 25, 2023

alexstine left a comment

alexstine Sep 26, 2023

alexstine Sep 26, 2023

alexstine left a comment

andrewserong commented Sep 27, 2023

jameskoster commented Sep 27, 2023

n2erjo00 commented Sep 27, 2023

n2erjo00 commented Sep 30, 2023

andrewserong left a comment

jameskoster commented Oct 2, 2023

n2erjo00 commented Nov 6, 2023

n2erjo00 commented Feb 22, 2024

github-actions bot commented Feb 22, 2024

	const { headingCount, paragraphCount, numberOfBlocks } = useSelect(
	( select ) => {
	const { getGlobalBlockCount } = select( blockEditorStore );
	return {
	headingCount: getGlobalBlockCount( 'core/heading' ),
	paragraphCount: getGlobalBlockCount( 'core/paragraph' ),
	numberOfBlocks: getGlobalBlockCount(),
	};
	},
	[]
	);

	id={ `edit-post-editor-list-view-overview-outline-${ instanceId }` }
	id={ instanceId }

	const instanceId = useInstanceId( ListViewOutline );
	const instanceId = useInstanceId( ListViewOutline, 'edit-post-editor-list-view-overview-outline' );

Update/list view outline information about post #50414

Are you sure you want to change the base?

Update/list view outline information about post #50414

Conversation

n2erjo00 commented May 6, 2023 • edited Loading

What?

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

alexstine left a comment

Choose a reason for hiding this comment

n2erjo00 commented May 11, 2023

alexstine left a comment

Choose a reason for hiding this comment

alexstine May 22, 2023

Choose a reason for hiding this comment

Mamaduka May 23, 2023

Choose a reason for hiding this comment

Mamaduka commented May 23, 2023

afercia commented Jul 11, 2023

alexstine commented Jul 14, 2023

afercia commented Jul 18, 2023

alexstine commented Jul 18, 2023

n2erjo00 commented Jul 22, 2023

afercia commented Jul 24, 2023

alexstine commented Jul 24, 2023

n2erjo00 commented Sep 24, 2023

alexstine commented Sep 24, 2023

n2erjo00 commented Sep 25, 2023

alexstine left a comment

Choose a reason for hiding this comment

alexstine Sep 26, 2023

Choose a reason for hiding this comment

alexstine Sep 26, 2023

Choose a reason for hiding this comment

alexstine left a comment

Choose a reason for hiding this comment

andrewserong commented Sep 27, 2023

jameskoster commented Sep 27, 2023

n2erjo00 commented Sep 27, 2023

n2erjo00 commented Sep 30, 2023

andrewserong left a comment

Choose a reason for hiding this comment

jameskoster commented Oct 2, 2023

n2erjo00 commented Nov 6, 2023

n2erjo00 commented Feb 22, 2024

github-actions bot commented Feb 22, 2024

n2erjo00 commented May 6, 2023 •

edited

Loading